feat(contacts): crop profile picture before saving by hansimgamr · Pull Request #397 · Avi0n/MeshCoreOne

hansimgamr · 2026-08-12T00:54:09Z

Description

Adds a crop step to the contact avatar picker flow. Previously, an image picked via the photo library or file importer was saved directly as the contact's profile picture with no way to reframe it. This PR inserts a "Move and Scale" screen between picking and saving, so the user can pan/zoom the image within a circular guide before it's committed.

Overview of Changes

New AvatarCropView: a full-screen pan/pinch-to-zoom crop UI with a circular guide, presented via fullScreenCover.
ContactDetailView: both avatar-import paths (photo picker and file importer) now route through a new presentCropSheet(data:) step instead of calling saveAvatar directly; the crop view's onComplete callback re-encodes the cropped result as JPEG and proceeds to the existing save path unchanged.
New localized strings for the crop screen's title/cancel/choose actions (English base locale; other locales unaffected).

Testing

Built and ran on iOS Simulator (iPhone 17 Pro Max, iOS 26.5) and on a physical iPhone 17 Pro Max.
Manually exercised the crop flow via both entry points (photo library picker and file importer): pan, pinch-to-zoom (clamped 1x-4x), cancel, and confirm/save, then verified the saved avatar reflects the cropped region.
swiftlint lint passes clean.
Full MC1Tests suite passes locally, with two pre-existing failures unrelated to this change (confirmed identical on dev tip, no diff touches these files): a naming-shadow compile issue in MessageLinkTokenizerTests.swift, and two URLSafetyCheckerTests cases that require live DNS resolution unavailable in this sandbox.

Tested on

iOS 26.5 (Simulator, iPhone 17 Pro Max) and physical iPhone 17 Pro Max

Checklist

This PR was discussed with the maintainer either via GitHub issue or other means (Also check if this PR is small enough not to need discussion e.g. typo fix)
I have read CONTRIBUTING.md
Testing steps are documented above.
This change is not low effort and I took the time to test it

Adds a pan/pinch crop step between picking a photo (library or file import) and saving it as a contact's avatar, instead of using the picked image as-is. Crops via UIImage.draw(in:) rather than raw CGImage cropping so EXIF-rotated photos crop the same region shown in the on-screen preview.

The committed generated file didn't match current swiftgen output ordering, tripping the codegen CI check.

hansimgamr · 2026-08-12T01:06:09Z

Note on CI: SPM Package Tests failed on `ConnectionManagerReconnectAbandonmentTests.swift` ("watchdog natural exit nils the task and preserve re-arms"), asserting on reconnection-watchdog task/generation state. This file isn't touched by this PR (diff only spans `AvatarCropView.swift`, `ContactDetailView.swift`, and localization/generated strings), and it was last modified by unrelated BLE reconnection commits. Looks like a timing-sensitive/flaky assertion rather than a regression from this change — flagging in case a re-run is needed.

edit: all tests pass

Code Generation Checks, Formatting and Linting, and Xcode Build all pass.

Avi0n

Thanks for this PR, nice feature add. AI code review below:

Avi0n · 2026-08-14T01:08:30Z

+      errorMessage = L10n.Contacts.Contacts.Detail.Avatar.invalidImage
+      return
+    }
+    avatarImageToCrop = image


presentCropSheet sets showAvatarCropSheet = true in the same turn as the PhotosPicker / fileImporter dismissal. This file already documents that presenting one modal while another is still dismissing is a race (showRepeaterAdminAuth waits for onDismiss before setting activeSheet). The crop cover can fail to appear, especially on the file-importer path where Data(contentsOf:) returns before the importer animation ends. The author tested both entry points, so this may be intermittent rather than always broken.

Suggestion: Hold the decoded image as pending state. Present the cover only after the picker/importer isPresented flag becomes false (or after an onDismiss). Prefer fullScreenCover(item:) with an Identifiable wrapper so the cover cannot open empty.

Avi0n · 2026-08-14T01:08:30Z

    }
  }

+  private func presentCropSheet(data: Data) {


presentCropSheet calls UIImage(data:) on the main actor and stores the result in @State. A 12–48MP camera photo can hitch the UI and hold ~100–200MB while the crop screen is open. The old path decoded inside Task.detached in processAvatarImage and immediately downscaled to 512pt. This is a regression on memory and main-thread work. ImageURLDetector.downsampledImage(from:) already shows the ImageIO thumbnail pattern used elsewhere in the app.

Suggestion: Decode off the main actor. Downsample with ImageIO to a display-sized max pixel dimension before assigning avatarImageToCrop. Crop from that bounded image (512–1024px is enough; processAvatarImage already caps at 512). Nil avatarImageToCrop when the cover dismisses.

Avi0n · 2026-08-14T01:08:30Z

+          onCancel: { showAvatarCropSheet = false },
+          onComplete: { cropped in
+            showAvatarCropSheet = false
+            Task { await saveAvatar(data: cropped.jpegData(compressionQuality: 0.9) ?? Data()) }


Confirm re-encodes the crop as JPEG at quality 0.9 and 1024px, then saveAvatar → processAvatarImage decodes it again, scales to 512, and re-encodes at 0.8. The first JPEG is thrown away. jpegData(...) ?? Data() also turns a failed encode into empty data; processAvatarImage then fails with invalidImage.

Suggestion: Pass the UIImage (or PNG/raw pixel data) into the existing processor, or render the crop at 512px and skip the extra JPEG. If jpegData is nil, set errorMessage and do not call saveAvatar.

Avi0n · 2026-08-14T01:08:30Z

+  }
+
+  private var magnificationGesture: some Gesture {
+    MagnificationGesture()


MagnificationGesture is deprecated as of iOS 17 (MagnifyGesture replaces it). Deployment is iOS 18, so this will warn. During the pinch, scale * pinchDelta is not clamped to 1...4, so the live preview can shrink below fill and show empty area inside the circle until onEnded.

Suggestion: Switch to MagnifyGesture. Clamp the live factor the same way onEnded does: min(max(scale * pinchDelta, 1), 4). Put those limits on named constants next to cropSize.

Avi0n · 2026-08-14T01:08:30Z

+
+  var body: some View {
+    NavigationStack {
+      GeometryReader { _ in


GeometryReader { _ in ignores its proxy. The inner dimmingMask already has its own GeometryReader. The outer reader only forces expansion that .frame(maxWidth: .infinity, maxHeight: .infinity) already provides.

Suggestion: Remove the outer GeometryReader and keep the ZStack plus the max frame.

Fixes the modal-presentation race between the picker/importer sheets and the crop cover, decodes and downsamples off the main actor via ImageIO instead of full-resolution UIImage(data:), removes the redundant JPEG re-encode by passing the cropped UIImage straight to the existing avatar processor, swaps the deprecated MagnificationGesture for MagnifyGesture with a clamped live pinch value, and drops an unused outer GeometryReader. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

hansimgamr · 2026-08-15T16:37:55Z

Pushed a follow-up commit addressing all 5 review comments:

Modal-presentation race: decoded images now go into a pending state and the crop cover only presents once the picker/importer's isPresented flips to false (via onChange), same pattern as showRepeaterAdminAuth/onDismiss elsewhere in this file. Also switched to fullScreenCover(item:) so it can't open empty.
Main-thread decode / memory: image decode now happens off the main actor via Task.detached, downsampled through ImageIO to a 1024px bound (same thumbnail technique as ImageURLDetector.downsampledImage(from:)) instead of a full-resolution UIImage(data:).
Double JPEG re-encode: the crop view now hands its UIImage straight to saveAvatar/processAvatarImage, which encodes once at 512px/0.8 quality instead of encoding at 1024px/0.9 and then re-encoding. This also removes the jpegData(...) ?? Data() fallback that could silently save empty avatar data.
Deprecated MagnificationGesture: switched to MagnifyGesture (deployment target is iOS 18), and the live pinch value is now clamped to 1...4x via named constants, so the preview can no longer shrink below fill mid-pinch.
Redundant outer GeometryReader: removed; the ZStack + .frame(maxWidth: .infinity, maxHeight: .infinity) already provided the expansion.

Verified xcodebuild build succeeds and swiftlint lint is clean on both changed files. Confirmed the MessageLinkTokenizerTests compile failure is pre-existing and unrelated (reproduces identically with these changes stashed out).

Avi0n

Thanks!

hansimgamr added 2 commits August 11, 2026 20:25

fix: regenerate L10n.swift with swiftgen

57aa27d

The committed generated file didn't match current swiftgen output ordering, tripping the codegen CI check.

hansimgamr force-pushed the feature/avatar-crop branch from 2c6bf03 to 57aa27d Compare August 12, 2026 00:56

chore: retrigger CI

2ea581c

Avi0n requested changes Aug 14, 2026

View reviewed changes

hansimgamr requested a review from Avi0n August 15, 2026 16:45

Avi0n approved these changes Aug 20, 2026

View reviewed changes

Avi0n merged commit 80f410a into Avi0n:dev Aug 20, 2026
4 checks passed

Navigation Menu

Uh oh!

feat(contacts): crop profile picture before saving - #397

feat(contacts): crop profile picture before saving#397
Avi0n merged 4 commits into
Avi0n:devfrom
hansimgamr:feature/avatar-crop

hansimgamr commented Aug 12, 2026 •

edited

Loading

Uh oh!

hansimgamr commented Aug 12, 2026 •

edited

Loading

Uh oh!

Avi0n left a comment

Uh oh!

Avi0n Aug 14, 2026

Uh oh!

Avi0n Aug 14, 2026

Uh oh!

Avi0n Aug 14, 2026

Uh oh!

Avi0n Aug 14, 2026

Uh oh!

Avi0n Aug 14, 2026

Uh oh!

hansimgamr commented Aug 15, 2026

Uh oh!

Avi0n left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

hansimgamr commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Overview of Changes

Testing

Tested on

Checklist

Uh oh!

hansimgamr commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Avi0n left a comment

Choose a reason for hiding this comment

Uh oh!

Avi0n Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

Avi0n Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

Avi0n Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

Avi0n Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

Avi0n Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

hansimgamr commented Aug 15, 2026

Uh oh!

Avi0n left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

hansimgamr commented Aug 12, 2026 •

edited

Loading

hansimgamr commented Aug 12, 2026 •

edited

Loading